[FSSDK-12148] Event retry adjustment #398

junaed-optimizely · 2026-01-19T17:58:19Z

Summary

This pull request introduces a standardized retry and exponential backoff mechanism for event dispatching across the Optimizely SDKs. The changes ensure that failed event dispatches are retried up to three times with increasing delays, improving reliability in the face of transient network or server errors.

Test plan

Existing tests should pass

Issues

FSSDK-12148

muzahidul-opti

Looks good, made a couple of comments

muzahidul-opti · 2026-01-20T16:27:01Z

OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs

+        ///     Default constructor using the shared static HttpClient.
        /// </summary>
-        static HttpClientEventDispatcher45()
+        public HttpClientEventDispatcher45() : this(null)


Why null ? Relevalent comment could be made.

This is constructor chaining in C#. this(null) will call the internal HttpeClientEventDispatcher45(HttpClient httpClient) which accepts httpClient as argument.

muzahidul-opti · 2026-01-20T16:37:14Z

OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs

+                    if (shouldRetry && attemptNumber < maxAttempts - 1)
+                    {
+                        await Task.Delay(backoffMs).ConfigureAwait(false);
+                        backoffMs = Math.Min(EventRetryConfig.MAX_BACKOFF_MS,


Could you please cross check the backoff calculations?

backoffMs = min(maxBack, backOffMs * pow(BACKOFF_MULTIPLIER, attempNumber))

Yeah. Both derives same result. 200, 400, 800 etc...

muzahidul-opti · 2026-01-20T16:42:11Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

+
+                    if (shouldRetry && attemptNumber < maxAttempts - 1)
+                    {
+                        Thread.Sleep(backoffMs);


Why don't we implement like the previous one.
await Task.Delay(backoffMs).ConfigureAwait(false);

So WebRequestEventDispatcher is for old platforms (.NET3.5, .NET4.0) and Task is not supported in 3.5 and for 4.0 no async/await support.

Uggg. We need to get <4.8 off our backs.

muzahidul-opti · 2026-01-20T16:46:08Z

OptimizelySDK/Odp/OdpEventManager.cs

+                    if (shouldRetry && attemptNumber < maxAttempts - 1)
+                    {
+                        Thread.Sleep(backoffMs);
+                        backoffMs = Math.Min(EventRetryConfig.MAX_BACKOFF_MS,


See the commnets☝🏻

Yeah! Same as the formula.

Copilot

Pull request overview

This pull request introduces a standardized retry and exponential backoff mechanism for event dispatching across the Optimizely C# SDK. The changes implement automatic retries (up to 3 total attempts) with exponential backoff (starting at 200ms, doubling to max 1000ms) for failed event dispatches on 5xx server errors and network failures.

Changes:

Created EventRetryConfig class with centralized retry constants (max 2 retries, 200ms initial backoff, 1000ms max backoff, 2.0x multiplier)
Refactored WebRequestEventDispatcher35 and HttpClientEventDispatcher45 to implement retry logic with exponential backoff
Updated OdpEventManager to use the new EventRetryConfig constants and enhanced error logging
Removed MAX_RETRIES constant from ODP Constants class in favor of centralized configuration
Added comprehensive unit tests for HttpClientEventDispatcher45 covering success, retry, and backoff scenarios

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
OptimizelySDK/Event/EventRetryConfig.cs	New configuration class defining retry constants (max retries, backoff timing)
OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs	Refactored dispatcher with retry logic and exponential backoff for .NET 4.5+
OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs	Refactored dispatcher with retry logic for .NET 3.5/4.0
OptimizelySDK/Odp/OdpEventManager.cs	Updated to use EventRetryConfig constants and improved error logging
OptimizelySDK/Odp/Constants.cs	Removed MAX_RETRIES constant (moved to EventRetryConfig)
OptimizelySDK.Tests/EventTests/HttpClientEventDispatcher45Test.cs	New comprehensive test suite for retry behavior and backoff timing
OptimizelySDK.Tests/OdpTests/OdpEventManagerTests.cs	Updated test timeout to accommodate backoff delays
Various .csproj files	Added EventRetryConfig.cs to project configurations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T17:47:30Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

-                var data = responseReader.ReadToEnd();
+                        // Success - exit the retry loop
+                        return;
+                    }


If the response status code is not HttpStatusCode.OK (e.g., 201 Created, 204 No Content, or any 4xx error that doesn't throw an exception), the code falls through without returning or incrementing attemptNumber, creating an infinite loop. The code should either handle non-OK success codes (2xx), or increment attemptNumber and continue the loop, or return after logging.

Suggested change

}

}

else

{

// Handle non-OK responses without exceptions using the same retry logic.

var shouldRetry = ShouldRetry(response.StatusCode);

if (shouldRetry && attemptNumber < maxAttempts - 1)

{

Thread.Sleep(backoffMs);

backoffMs = Math.Min(EventRetryConfig.MAX_BACKOFF_MS,

(int)(backoffMs * EventRetryConfig.BACKOFF_MULTIPLIER));

attemptNumber++;

}

else

{

LogMessage(

LogLevel.ERROR,

string.Format(

"Error Dispatching Event after {0} attempt(s): received HTTP status code {1}",

attemptNumber + 1,

(int)response.StatusCode));

return;

}

}

Copilot · 2026-01-20T17:47:31Z

OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs

+                    var request = new HttpRequestMessage
+                    {
+                        RequestUri = new Uri(logEvent.Url),
+                        Method = HttpMethod.Post,
+                        // The Content-Type header applies to the Content, not the Request itself
+                        Content =
+                            new StringContent(json, Encoding.UTF8, "application/json"),
+                    };

-                foreach (var h in logEvent.Headers)
-                {
-                    if (h.Key.ToLower() != "content-type")
+                    foreach (var h in logEvent.Headers)
                    {
-                        request.Content.Headers.Add(h.Key, h.Value);
+                        if (h.Key.ToLower() != "content-type")
+                        {
+                            request.Content.Headers.Add(h.Key, h.Value);
+                        }
                    }


The HttpRequestMessage and its Content (StringContent) are not explicitly disposed. While HttpClient may handle the request disposal, the Content should be explicitly disposed to prevent resource leaks. Consider wrapping the request creation in a using statement or ensuring disposal in a finally block.

Copilot · 2026-01-20T17:47:31Z

OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs

+                    // For non-HTTP exceptions, log and don't retry
+                    Logger.Log(LogLevel.ERROR, $"Error Dispatching Event: {ex.GetAllMessages()}");
+                    return;
+                }


The HttpResponseMessage is not disposed after use, which can lead to resource leaks. The response object should be disposed in a finally block or by using a using statement to ensure proper cleanup of HTTP connections.

Suggested change

}

}

finally

{

response?.Dispose();

}

Copilot · 2026-01-20T17:47:31Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

    public class WebRequestClientEventDispatcher35 : IEventDispatcher
    {
-        // TODO Catch and Log Errors
-        public Logger.ILogger Logger { get; set; }
-
-        private HttpWebRequest Request = null;
+        public ILogger Logger { get; set; }

        /// <summary>
-        /// Dispatch the Event
-        /// The call will not wait for the result, it returns after sending (fire and forget)
-        /// But it does get called back asynchronously when the response comes and handles
+        ///     Dispatch the Event with retry and exponential backoff.
+        ///     The call will not wait for the result, it returns after sending (fire and forget)
+        ///     But it does get called back asynchronously when the response comes and handles
        /// </summary>
        /// <param name="logEvent"></param>
        public void DispatchEvent(LogEvent logEvent)
        {
-            Request = (HttpWebRequest)WebRequest.Create(logEvent.Url);
+            ThreadPool.QueueUserWorkItem(_ => DispatchWithRetry(logEvent));
+        }

-            Request.UserAgent = "Optimizely-csharp-SDKv01";
-            Request.Method = logEvent.HttpVerb;
+        /// <summary>
+        ///     Dispatch event with retry logic and exponential backoff.
+        /// </summary>
+        private void DispatchWithRetry(LogEvent logEvent)
+        {
+            var attemptNumber = 0;
+            var backoffMs = EventRetryConfig.INITIAL_BACKOFF_MS;
+            var maxAttempts = 1 + EventRetryConfig.MAX_RETRIES;

-            foreach (var h in logEvent.Headers)
+            while (attemptNumber < maxAttempts)
            {
-                if (!WebHeaderCollection.IsRestricted(h.Key))
+                HttpWebRequest request = null;
+                HttpWebResponse response = null;
+                try
                {
-                    Request.Headers[h.Key] = h.Value;
-                }
-            }
+                    request = (HttpWebRequest)WebRequest.Create(logEvent.Url);
+                    request.UserAgent = "Optimizely-csharp-SDKv01";
+                    request.Method = logEvent.HttpVerb;

-            Request.ContentType = "application/json";
+                    foreach (var h in logEvent.Headers)
+                    {
+                        if (!WebHeaderCollection.IsRestricted(h.Key))
+                        {
+                            request.Headers[h.Key] = h.Value;
+                        }
+                    }

-            using (var streamWriter = new StreamWriter(Request.GetRequestStream()))
-            {
-                streamWriter.Write(logEvent.GetParamsAsJson());
-                streamWriter.Flush();
-                streamWriter.Close();
-            }
+                    request.ContentType = "application/json";

-            var result =
-                Request.BeginGetResponse(new AsyncCallback(FinaliseHttpAsyncRequest), this);
-        }
+                    using (var streamWriter = new StreamWriter(request.GetRequestStream()))
+                    {
+                        streamWriter.Write(logEvent.GetParamsAsJson());
+                        streamWriter.Flush();
+                        streamWriter.Close();
+                    }

-        private static void FinaliseHttpAsyncRequest(IAsyncResult result)
-        {
-            var _this = (WebRequestClientEventDispatcher35)result.AsyncState;
-            _this.FinalizeRequest(result);
-        }
+                    response = (HttpWebResponse)request.GetResponse();

-        private void FinalizeRequest(IAsyncResult result)
-        {
-            var response = (HttpWebResponse)Request.EndGetResponse(result);
+                    if (response.StatusCode == HttpStatusCode.OK)
+                    {
+                        using (var responseStream = response.GetResponseStream())
+                        using (var responseReader =
+                               new StreamReader(responseStream, Encoding.UTF8))
+                        {
+                            responseReader.ReadToEnd();
+                        }

-            if (response.StatusCode == HttpStatusCode.OK)
-            {
-                // Read the results, even though we don't need it.
-                var responseStream = response.GetResponseStream();
-                var streamEncoder = System.Text.Encoding.UTF8;
-                var responseReader = new StreamReader(responseStream, streamEncoder);
-                var data = responseReader.ReadToEnd();
+                        // Success - exit the retry loop
+                        return;
+                    }
+                }
+                catch (WebException ex)
+                {
+                    var httpResponse = ex.Response as HttpWebResponse;
+                    var shouldRetry = ShouldRetry(httpResponse?.StatusCode);
+
+                    if (shouldRetry && attemptNumber < maxAttempts - 1)
+                    {
+                        Thread.Sleep(backoffMs);
+                        backoffMs = Math.Min(EventRetryConfig.MAX_BACKOFF_MS,
+                            (int)(backoffMs * EventRetryConfig.BACKOFF_MULTIPLIER));
+                        attemptNumber++;
+                    }
+                    else
+                    {
+                        LogMessage(LogLevel.ERROR,
+                            string.Format("Error Dispatching Event after {0} attempt(s): {1}",
+                                attemptNumber + 1, ex.GetAllMessages()));
+                        return;
+                    }
+                }
+                catch (Exception ex)
+                {
+                    // For non-web exceptions, log and don't retry
+                    LogMessage(LogLevel.ERROR, "Error Dispatching Event: " + ex.GetAllMessages());
+                    return;
+                }
+                finally
+                {
+                    response?.Close();
+                }
            }
-            else
+        }
+
+        /// <summary>
+        ///     Determines whether a request should be retried based on HTTP status code.
+        ///     Retries on 5xx server errors and network failures (null status code).
+        /// </summary>
+        private static bool ShouldRetry(HttpStatusCode? statusCode)
+        {
+            // Retry on network failures (no response)
+            if (statusCode == null)
            {
-                // TODO: Add Logger and capture exception
-                //throw new Exception(string.Format("Response Not Valid {0}", response.StatusCode));
+                return true;
            }
+
+            // Retry on 5xx server errors
+            var code = (int)statusCode.Value;
+            return code >= 500 && code < 600;
+        }
+
+        /// <summary>
+        ///     Helper method to log messages safely when Logger might be null.
+        /// </summary>
+        private void LogMessage(LogLevel level, string message)
+        {
+            Logger?.Log(level, message);
        }
    }
 }


The WebRequestClientEventDispatcher35 class has been significantly refactored with new retry logic, but there are no tests covering this implementation. Since HttpClientEventDispatcher45 has comprehensive tests including retry scenarios, exponential backoff timing, and various error conditions, the WebRequestClientEventDispatcher35 should have equivalent test coverage to ensure the retry logic works correctly on .NET 3.5/4.0 platforms.

Copilot · 2026-01-20T17:47:31Z

OptimizelySDK/Event/Dispatcher/HttpClientEventDispatcher45.cs

+                    foreach (var h in logEvent.Headers)
                    {
-                        request.Content.Headers.Add(h.Key, h.Value);
+                        if (h.Key.ToLower() != "content-type")
+                        {
+                            request.Content.Headers.Add(h.Key, h.Value);
+                        }
                    }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2026-01-20T17:47:32Z

OptimizelySDK.Tests/EventTests/HttpClientEventDispatcher45Test.cs

+                var response = new HttpResponseMessage(statusCode)
+                {
+                    Content = new StringContent("{}"),
+                };


Disposable 'HttpResponseMessage' is created but not disposed.

mikechu-optimizely

Implementation looks good. Small suggestion below.

mikechu-optimizely · 2026-01-20T17:48:13Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

-                    Request.Headers[h.Key] = h.Value;
-                }
-            }
+                    request = (HttpWebRequest)WebRequest.Create(logEvent.Url);


Consider using as and checking for null over type coercion.

mikechu-optimizely · 2026-01-20T17:49:53Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

+
+                    if (shouldRetry && attemptNumber < maxAttempts - 1)
+                    {
+                        Thread.Sleep(backoffMs);


Uggg. We need to get <4.8 off our backs.

mikechu-optimizely · 2026-01-20T17:51:51Z

OptimizelySDK/Event/Dispatcher/WebRequestEventDispatcher35.cs

-            var _this = (WebRequestClientEventDispatcher35)result.AsyncState;
-            _this.FinalizeRequest(result);
-        }
+                    response = (HttpWebResponse)request.GetResponse();


as here too perhaps?

junaed-optimizely added 3 commits January 19, 2026 23:22

[FSSDK-12148] event retry addition

440803b

[FSSDK-12148] format fix

4c3c80b

[FSSDK-12148] class definition fix

ab3efea

optimizely-codereview-bot assigned junaed-optimizely Jan 19, 2026

junaed-optimizely added 2 commits January 20, 2026 21:40

[FSSDK-12148] test addition

cb6fe56

[FSSDK-12148] format fix

ab38fe3

muzahidul-opti requested changes Jan 20, 2026

View reviewed changes

mikechu-optimizely requested a review from Copilot January 20, 2026 17:42

Copilot started reviewing on behalf of mikechu-optimizely January 20, 2026 17:43 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

mikechu-optimizely approved these changes Jan 20, 2026

View reviewed changes

-                }
+                }
+                finally
+                {
+                    response?.Dispose();
+                }

[FSSDK-12148] Event retry adjustment #398

Are you sure you want to change the base?

[FSSDK-12148] Event retry adjustment #398

Conversation

junaed-optimizely commented Jan 19, 2026

Summary

Test plan

Issues

Uh oh!

muzahidul-opti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

mikechu-optimizely left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants